Support Content Exclusion for Copilot Hover #13143
Conversation
…nto dev/spebl/content_ex
…nto dev/spebl/content_ex
spebl
added
the
Feature: Copilot Hover
We're still waiting for final sign-off before rolling it out. @benmcmorran has been tracking the privacy approval which is the last step. |
|
@sean-mcmanus Not sure exactly when you were planning to do the 1.23 official release, but assuming we get the approval from the VS Code team to use the proposed API soon, is it feasible to ensure this PR makes it into the release? That will put us in a position to enable for the GA audience once all sign-offs are complete. |
|
Discussed offline with @Colengms and it sounds like that timeline may not be feasible. It's fine to hold this for the next insiders release instead. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
These changes, about to push an update. |
| } | ||
| } catch (err) { | ||
| if (err instanceof Error) { | ||
| await reportCopilotFailure(copilotHoverProvider, hoverDocument, hoverPosition, err.message); |
There was a problem hiding this comment.
Could this err.message contain PII, such as the user name and paths?
There was a problem hiding this comment.
Good call that logging any generic error could have that issue. Perhaps something like "Copilot Hover Error: " + err.name? Should be safe from PII but it would be good to be able to track what type of errors are hit.
There was a problem hiding this comment.
Yeah, I used err.name in my PR at #13158 .
There was a problem hiding this comment.
I wasn't sure how to determine if vscode.LanguageModelError should be handled (which has code) versus Error (which only has name).
There was a problem hiding this comment.
The telemetry already is sent via telemetry.logLanguageServerEvent("CopilotHoverError", { "ErrorMessage": so adding "Copilot Hover Error: " doesn't seem to add any info.
Get the files used for the context generation and bail before calling the LM if they're excluded.